Skip to content

Conversation

@dthampy
Copy link
Contributor

@dthampy dthampy commented Jun 3, 2025

Description

It turns out that during aio app dev, any input param whose value is left as literal placeholders like for example $<MY_FIRSTINPUT> ends up being injected verbatim in the environment instead of being skipped if it is not mapped in the process env

Fix:

To align with the expectation that an undefined env variable should result in a empty string for the action input, the interpolate function was modified so that the it would now check if the property exists in the process.env and if not, it returns an empty string

Related Issue

ACNA-3846
Original git hub issue here

Motivation and Context

It turns out that during aio app dev, any input param whose value is left as literal placeholders like for example ${FIRST_INPUT}, $SECOND_INPUT, or {THIRD_INPUT} end up being injected verbatim in the environment instead of being skipped if it is not mapped in the process env

How Has This Been Tested?

  • Locally linked the plugin - aio plugins link .
  • Made the appropriate change to the app.config.yaml file for an action ( as described in the bug) and make sure that the variable is not defined in the process env
  • Once built, invoke the action and verify that instead of the literal placeholder value is returned, an empty string is returned
  • If the value is set in the env, that will be used.
  • Verified that the all the tests are passing

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I have signed the Adobe Open Source CLA.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@codecov
Copy link

codecov bot commented Jun 3, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (afad051) to head (40664e8).
Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #154   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            7         7           
  Lines          647       649    +2     
  Branches       132       133    +1     
=========================================
+ Hits           647       649    +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@purplecabbage
Copy link
Member

It is completely valid to send an input of "{THIRD_INPUT}" to an action, the fact that it looks like something that should be replaced does not mean we can simply make it an empty string. This also applies to much more than just the dev command ...

@dthampy
Copy link
Contributor Author

dthampy commented Jun 3, 2025

It is completely valid to send an input of "{THIRD_INPUT}" to an action, the fact that it looks like something that should be replaced does not mean we can simply make it an empty string. This also applies to much more than just the dev command ...

Case 1: If you want to send the EXACT string "{MY_VAR}" or "$MY_VAR" (or any string containing these) to your action:
The code looks for patterns like $VAR, ${VAR}, or {VAR} and wouldn't touch these patterns if they are immediately surrounded by quote characters (like " or ') within the value string itself.

Case 2: If the intention is to use an actual environment variable (For example: if you want $MY_ENV_VAR to be replaced by the value of the MY_ENV_VAR .env variable), then you'd write something like this (unquoted within the value string)

 myConfigInput: $MY_ENV_VAR
 mysecondConfigInput: ${ANOTHER_ENV_VAR}

All this fix does is, if it sees "bare"( or above pattern) placeholders
(i) If MY_ENV_VAR is NOT set in your system environment, aio app dev will now pass an empty string ("") for myConfigInput.
(ii) if If MY_ENV_VAR is set in your actual system environment, its value is put in.
Our fix makes local behavior match the deployed behavior (empty string) for that specific case ( Please see 750

@purplecabbage if you think that the the original issue is not a bug, we can leave a comment in there close it as not a bug.

@purplecabbage purplecabbage requested a review from Copilot June 3, 2025 22:39
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR resolves the bug where unmapped action input parameters were injected as literal placeholders in the environment by modifying the interpolation logic.

  • Updated the test expectation in run-dev.test.js to expect an empty string when the property is missing.
  • Modified the interpolate function in run-dev.js to check for the existence of the property and return an empty string if it does not exist.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
test/lib/run-dev.test.js Updated expected output for unmapped parameters to return an empty string.
src/lib/run-dev.js Refactored the interpolate function to return an empty string for unmapped variables.

@purplecabbage
Copy link
Member

It is completely valid to send an input of "{THIRD_INPUT}" to an action, the fact that it looks like something that should be replaced does not mean we can simply make it an empty string. This also applies to much more than just the dev command ...

I misunderstood ... ignore this

@dthampy dthampy merged commit 32dbd50 into main Jun 6, 2025
10 checks passed
@dthampy dthampy deleted the ACNA-3846-handle-input-param branch June 6, 2025 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants